fix: give a same-service window.open a real window, not nil - #18
Open
marcioviniciusspiridigliozzi-dot wants to merge 1 commit into
Open
Conversation
A new-window request whose target belongs to the opening service was
collapsed into the opener's web view, and the delegate returned nil.
For a link click that is right, and it has to stay that way: Slack's
target=_blank workspace links go through this path and should keep
loading in place rather than spawning a window each time.
For a programmatic window.open() it is wrong. The caller gets a window
handle back, and callers test it:
const w = window.open(url); if (!w) return;
Returning nil reads as "popup blocked", so the page abandons whatever it
was starting with no window and no error to show for it. Nothing in the
UI says a request was refused, which makes it a hard thing to diagnose
from the outside.
So narrow the collapse to .linkActivated rather than removing it. A
same-service window.open now falls through to a real window, which
shares the opener's data store, so a session started in it lands in the
right place.
The rule moves into shouldLoadNewWindowInPlace so it can be tested
without a live WKWebView.
Scope, stated plainly: I first reached this while chasing a Teams
sign-in failure and believed it was the cause. It was not — logging
createWebViewWith showed it was never called during that failure, and
the real cause was ITP blocking a third-party cookie, which I have sent
separately. This change stands on its own as a correctness fix for
window.open; it is not a fix for that bug, and I would rather say so
than let the connection be assumed.
marcioviniciusspiridigliozzi-dot
force-pushed
the
fix/window-open-handle
branch
from
August 11, 2026 10:27
6fc5d44 to
e93e408
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A new-window request whose target belongs to the opening service was collapsed into the opener's web view, and the delegate returned
nil.For a link click that is right, and it has to stay that way — Slack's
target=_blankworkspace links go through this path and should keep loading in place rather than spawning a window each time.For a programmatic
window.open()it is wrong. The caller gets a window handle back, and callers test it:Returning
nilreads as "popup blocked", so the page abandons whatever it was starting with no window and no error to show for it. Nothing in the UI says a request was refused, which makes it a hard thing to diagnose from the outside.The change
Narrow the collapse to
.linkActivatedrather than removing it. A same-servicewindow.opennow falls through to a real window, which shares the opener's data store, so a session started in it lands in the right place.The rule moves into
shouldLoadNewWindowInPlaceso it can be tested without a liveWKWebView— covered for the clicked same-service link (still collapses), the programmatic same-service popup (gets a window), the cross-service case, and unknown hosts on either side.Scope, stated plainly
I first reached this while chasing a Teams sign-in failure and believed it was the cause. It was not. Logging
createWebViewWithshowed it was never called during that failure — zero times — and the real cause turned out to be ITP blocking a third-party cookie, which I have sent separately in #17.This change stands on its own as a correctness fix for
window.open. It is not a fix for that bug, and I would rather say so than let the connection be assumed from the timing of the two PRs.Note on CI
This branches from
main, which currently has an intermittentStoreRepairtest flake; I sent a fix for it in #15. A red run here is most likely that rather than this change.